-
Notifications
You must be signed in to change notification settings - Fork 4
CFM-103_LDAP_server_mvc #374
base: develop
Are you sure you want to change the base?
Conversation
c991c60 to
cd5cdbd
Compare
cd5cdbd to
c31a514
Compare
7a63755 to
05b735c
Compare
af9faba to
ea8ac7d
Compare
dd66827 to
4293765
Compare
1b2ec09 to
bf616a2
Compare
179d6da to
66621fa
Compare
4fe0400 to
ef65098
Compare
ef65098 to
d9b0ace
Compare
| * | ||
| * @param EventInterface $event Cake Event | ||
| * @return \Cake\Http\Response|null|void | ||
| * @since COmanage Registry v5.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v5.3.0
| return true; | ||
| } | ||
| } catch (\Exception $e) { | ||
| Log::error(__METHOD__ . "::Connection test '{$name}' failed: " . $e->getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use LabeledLogTrait::llog it'll automatically insert function call information in a standard way
| * @throws \InvalidArgumentException If non-delete and $newDn is missing. | ||
| * @throws \RuntimeException On LDAP failures not handled by fallback. | ||
| */ | ||
| public function applyProvisioningPlan( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this is a bit of a grey area, but I think there's a question as to how much of this belongs here and how much belongs in the LDAP Provisioner.
For comparison, the other Server models tend to be pretty thin, providing basic configuration and connectivity, and maybe functions that map pretty close to the wire protocol. For example, SmtpServer doesn't have any support functions (those are implemented in DeliveryUtilities), while MatchServer basically has function calls that perform the Match API requests.
| * Performs a case-insensitive suffix match. If $baseDn is empty or not a suffix | ||
| * of $newDn, returns $newDn unchanged. | ||
| */ | ||
| protected function computeRelativeRdn(string $newDn, string $baseDn): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Relative RDN" is like "ATM Machine" :)
| $context['diagnostic_message'] = $err; | ||
| } | ||
|
|
||
| Log::error(__METHOD__ . "::LDAP error during $functionName", $context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like above, maybe use LabeledLogTrait?
| if ($connection) { | ||
| $this->Flash->success(__d('operation', 'rs.test.ok')); | ||
| Log::debug("Successfully connected to server " . $serverId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LabeledLogTrait?
| try { | ||
| if (method_exists($pluginTable, 'checkConnectivity')) { | ||
| // Set the Connection Manager config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Connection Manager?
| 'controller' => 'Servers', | ||
| 'action' => 'index', | ||
| '?' => [ | ||
| 'co_id' => $serverObj->co_id | ||
| ] | ||
| ]; | ||
|
|
||
| // Redirect to the referring page, with a fallback to the Servers index | ||
| return $this->redirect($this->referer($fallbackRedirectToIndexPage)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use generateRedirect()?
| [ | ||
| 'action' => 'testconnection', | ||
| 'label' => __d('operation', 'connection.test'), | ||
| 'icon' => 'fact_check' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I think we're going to need to have a meeting to discuss icon selection
| msgstr "Failed to connect to LDAP server {0}" | ||
|
|
||
| msgid "error.LdapServers.credentials" | ||
| msgstr "Missing authentication credentials for LDAP server {0}" | ||
|
|
||
| msgid "error.LdapServers.serverurl.valid" | ||
| msgstr "Please enter a valid ldap or ldaps URL (e.g., ldaps://server:port)" | ||
|
|
||
| msgid "field.LdapServers.serverurl" | ||
| msgstr "Server URL" | ||
|
|
||
| msgid "field.LdapServers.serverurl.desc" | ||
| msgstr "LDAP server URL, e.g. ldap://ldap.example.org or ldaps://ldap.example.org" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The messages should be grouped together as the rest of the file is grouped, so error messages with other error messages, field messages with other field messages, etc.
…hods supports from the ldap server. Fixed ldap connection cache.
… Add more LdapCommon codes.
d9b0ace to
fdaf7c3
Compare
No description provided.